test: optimization of functional tests #6521
Conversation
WalkthroughThe pull request introduces modifications across three functional test files: 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
💤 Files with no reviewable changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (2)
🔇 Additional comments (7)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
test/functional/feature_mnehf.py (4)
28-28: Prefer adding a short explanatory comment for large numeric parameters.Using
999999999999conveys that the activation will happen almost indefinitely, but it might be helpful to comment on why such a large parameter is used (e.g., for test flexibility or to prevent actual activation during tests).
167-169: Suggestion: Replace arithmetic with direct constants for clarity.Writing
for _ in range(2)might be more readable thanfor _ in range(4 // 2)unless the4is a placeholder for dynamic usage.- for _ in range(4 // 2): + for _ in range(2): self.check_fork('started') self.generate(node, 2)
172-175: Clarify the intention behindrange(4 // 2)and conditional restarts.Consider replacing
range(4 // 2)withrange(2)and adding in-code comments detailing the rationale for restarting nodes on the second iteration.
198-200: Minor improvement: Clarify the relationship between block generation and node restarts.Generating 4 blocks, then restarting node 0, might benefit from a brief comment indicating why this sequence is necessary (e.g., to simulate a specific network condition).
test/functional/feature_llmq_chainlocks.py (1)
186-186: Inline comment recommended.Explain why a 5-second timeout is chosen here. This helps maintain clarity for future maintainers or runs in slower test environments.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
test/functional/feature_llmq_chainlocks.py(4 hunks)test/functional/feature_maxuploadtarget.py(0 hunks)test/functional/feature_mnehf.py(6 hunks)
💤 Files with no reviewable changes (1)
- test/functional/feature_maxuploadtarget.py
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Build Dependencies (linux64, x86_64-pc-linux-gnu)
- GitHub Check: Build Dependencies (arm-linux, arm-linux-gnueabihf)
🔇 Additional comments (7)
test/functional/feature_mnehf.py (4)
125-125: Log message aligns with the test's scenario.This logging statement clarifies assumptions for EHF signals pre-V20. No issues here.
186-187: No issues identified.Generating 4 blocks and checking fork status is consistent with the test flow. Good use of
check_fork('defined')to validate state.
225-225: Informative logging.Log messages help track the test flow and are especially crucial for debugging. Good practice.
229-233: Nicely encapsulated checking mechanism withwait_until.Introducing
check_ehf_activated(self)is a clear improvement over static sleep usage. Well done.test/functional/feature_llmq_chainlocks.py (3)
87-87: Introduced a shorter timeout.A 5-second timeout may be sufficient in most environments, but consider leaving a margin or making it configurable if network latency can vary significantly.
160-163: Good use ofwait_untilfor dynamic assertions.These changes replace sleep-based timing with condition-based waits, improving reliability. Ensure that large regressions in slow environments won't cause unwanted timeouts.
200-206: Condensed condition checks with a helper function.Defining
test_cb(self)clarifies the readiness condition. The logic is reversed (return Falseif chainlock is set), which is valid as long as the calling condition expects a boolean outcome. Good approach.
knst
force-pushed
the
functional-tests-speedup
branch
from
a267293 to
9bec526
Compare
|
Before after |
Issue being fixed or feature implemented
Functional tests running locally takes too long time; and limitation is not even CPU and RAM for many of them.
This PR helps to speed some of the functional tests.
What was done?
Not much has been done, but some of the low-hanging fruit has been picked:
feature_maxuploadtarget.pymuch fasterwait_untilwait_untilwith timeoutHow Has This Been Tested?
Before this PR:
With these optimizations:
CI finishes 2 minutes faster (for non-tsan):
https://gitlab.com/dashpay/dash/-/jobs/8739701280
https://gitlab.com/dashpay/dash/-/jobs/8776172870
Size of artefacts on CI is slightly smaller (4 MB smaller for ubsan): 759 -> 755MB.
Breaking Changes
N/A
Checklist: